Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement] fixup isNullable of cast #15380

Merged
merged 4 commits into from
Dec 21, 2022

Conversation

silverbullet233
Copy link
Contributor

@silverbullet233 silverbullet233 commented Dec 18, 2022

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Which issues of this PR fixes :

Fixes #

Problem Summary(Required) :

currently, isNullable always returns true for cast expr, which may not make sense. actually, if target type can be fully compitable with from type, we can keep the nullable property same as child operator.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr will affect users' behaviors
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto backported to target branch
    • 2.5
    • 2.4
    • 2.3
    • 2.2

@silverbullet233
Copy link
Contributor Author

run starrocks_admit_test

@silverbullet233
Copy link
Contributor Author

run starrocks_admit_test

@silverbullet233
Copy link
Contributor Author

run starrocks_admit_test

@silverbullet233
Copy link
Contributor Author

run starrocks_fe_unittest

satanson
satanson previously approved these changes Dec 19, 2022
kangkaisen
kangkaisen previously approved these changes Dec 19, 2022
@kangkaisen kangkaisen enabled auto-merge (squash) December 19, 2022 11:39
@wanpengfei-git wanpengfei-git added the Approved Ready to merge label Dec 19, 2022
@wanpengfei-git
Copy link
Collaborator

run starrocks_admit_test

@mofeiatwork
Copy link
Contributor

run starrocks_fe_unittest

mofeiatwork
mofeiatwork previously approved these changes Dec 19, 2022
@silverbullet233
Copy link
Contributor Author

run starrocks_fe_unittest

auto-merge was automatically disabled December 20, 2022 01:35

Head branch was pushed to by a user without write access

@github-actions github-actions bot removed the Approved Ready to merge label Dec 20, 2022
@github-actions github-actions bot removed the be-build label Dec 20, 2022
@sonarcloud
Copy link

sonarcloud bot commented Dec 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@wanpengfei-git
Copy link
Collaborator

[FE PR Coverage Check]

😍 pass : 6 / 6 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/sql/optimizer/operator/scalar/CastOperator.java 3 3 100.00% []
🔵 com/starrocks/analysis/CastExpr.java 3 3 100.00% []

@silverbullet233
Copy link
Contributor Author

run starrocks_admit_test

1 similar comment
@mofeiatwork
Copy link
Contributor

run starrocks_admit_test

@mofeiatwork mofeiatwork enabled auto-merge (squash) December 21, 2022 03:39
@mofeiatwork mofeiatwork merged commit 2d66dfe into StarRocks:main Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants